Fixes #39437 - Apply taxonomies to UI bulk actions by nadjaheitmann · Pull Request #11033 · theforeman/foreman

nadjaheitmann · 2026-06-22T08:36:54Z

The error is caused by a mismatch between what is shown in the UI and what the backend resolved as current organization/location.

The API request that is made from the frontend does not contain any information which organization/location the action should be performed on. It only sends an empty search string when an 'Select All' is activated. This can cause serious issue if for - whatever reason - there is a mismatch between the frontend location and the backend scope.
(this did not happen on the old All Hosts page - it would try to show the affected hosts beforehand and just show an empty page).

Even worse, the new page shows the correct number of hosts in the confirmation window, so there is no way for the user to know that something might go wrong!!!

We managed to reproduce the error when doing the following:

open two tabs with Foreman
load All Hosts page on one tab
select all hosts with the 'Select All' checkbox
go to other tab
select another location
go back to tab with All Hosts
fire an action that explicitly triggers the bulk hosts API, e.g. delete hosts or power state

(however, this does not work every time, it's a bit random)

nadjaheitmann · 2026-06-22T18:12:02Z

@jeremylenz @lfu @MariaAga This is a very serious issue and we are trying to find a solution. Any opinions?

MariaAga · 2026-06-23T09:50:36Z

Adding also @ianballou and @parthaa as they reviewed the original. Was there a reason not to scope by location, and only scope by org?

nadjaheitmann · 2026-06-23T11:20:14Z

Adding also @ianballou and @parthaa as they reviewed the original. Was there a reason not to scope by location, and only scope by org?

Thanks! We were investigating a bit more and sending search strings in general may result in other dangerous behavior. For example, you could:

Go to 'All hosts' page
Select all hosts
wait 20 minutes
delete all

In the 20 minutes that you were waiting, someone else creates 50 new hosts that would all be deleted. Looking at this, it looks bigger than adding 'Location' to the scope.

jeremylenz · 2026-06-23T12:58:26Z

Was there a reason not to scope by location, and only scope by org?

Probably not. We tend to neglect taxonomies other than org, since Katello isn't location-focused. This seems like a needed fix.

sending search strings in general may result in other dangerous behavior. For example, you could:

Go to 'All hosts' page

Select all hosts

wait 20 minutes

delete all

In the 20 minutes that you were waiting, someone else creates 50 new hosts that would all be deleted.

This part is just an effect of how scoped search works. Remote Execution has a feature that attempts to get around this by asking the user if you want the host query to be "dynamic" or "static". We could consider expanding that feature to other searches, but I don't consider it part of the scope here.

nadjaheitmann · 2026-06-23T13:12:28Z

Was there a reason not to scope by location, and only scope by org?

Probably not. We tend to neglect taxonomies other than org, since Katello isn't location-focused. This seems like a needed fix.

Do you think we need the complete path from frontend to backend or is it enough to have the validation part at the backend?

In the 20 minutes that you were waiting, someone else creates 50 new hosts that would all be deleted.

This part is just an effect of how scoped search works. Remote Execution has a feature that attempts to get around this by asking the user if you want the host query to be "dynamic" or "static". We could consider expanding that feature to other searches, but I don't consider it part of the scope here.

I agree that those are two different issues. Shall I add another issue on Redmine? If you look at the old 'All Hosts' page, it would give you a preview of all the hosts that it would operate bulk actions on. So this seems to be a regression. I know that the old page works on the controller data directly and moving all this data via API calls is tedious.

jeremylenz · 2026-06-23T13:21:50Z

Do you think we need the complete path from frontend to backend or is it enough to have the validation part at the backend?

Not sure I understand the question completely (I haven't reviewed the code in detail yet), but I would expect it to respect the UI location context selector the same as it does for org.

As for new Redmine issues, I don't think it's a regression that searches are not static, but you could feel free to add that as an RFE. What specific behavior do you see as a regression? We could add a modal or wizard step that shows you selected hosts, but that too would be dynamic. And I think we decided with @MariSvirik not to add it because you've just selected hosts, so it would be redundant to show you the hosts you've just selected.

nadjaheitmann · 2026-06-23T15:40:36Z

Do you think we need the complete path from frontend to backend or is it enough to have the validation part at the backend?

Not sure I understand the question completely (I haven't reviewed the code in detail yet), but I would expect it to respect the UI location context selector the same as it does for org.

In this PR, I added a part to the backend that copies the behavior of organization for location. I also added that both are explicitly passed from the frontend to the backend. Not sure whether we need that second part.

ianballou · 2026-06-23T17:16:37Z

Go to 'All hosts' page

Select all hosts

wait 20 minutes

delete all

A part of me thinks this is intended behavior. You wanted all of the hosts deleted - you selected "Select all". That means all the hosts will be deleted when you hit submit. The intention to destroy everything is there. If you linger on a form for too long, the intent hasn't gone away to delete all hosts.

Now, could we make that behavior easier to understand? Perhaps, yeah. But I think it's a good thing to shelf for later.

ianballou · 2026-06-23T17:50:50Z


      def_param_group :bulk_host_ids do
        param :organization_id, :number, :required => true, :desc => N_("ID of the organization")
+        param :location_id, :number, :required => true, :desc => N_("ID of the location")


Since this line is being added - neither the location nor the organization are actually required. We could probably take the required flag out for both.

If it is both supposed to be optional, we could even remove the complete line 12 and have it consistent with the rest of the API. Right now, we overwrite irganization_id param and use the default the location_id param description. This is how it looks right now:

I like the default "Set the location context.." better anyway. 👍

sbernhard · 2026-06-23T21:47:11Z

Go to 'All hosts' page

Select all hosts

wait 20 minutes

delete all

A part of me thinks this is intended behavior. You wanted all of the hosts deleted - you selected "Select all". That means all the hosts will be deleted when you hit submit. The intention to destroy everything is there. If you linger on a form for too long, the intent hasn't gone away to delete all hosts.

Now, could we make that behavior easier to understand? Perhaps, yeah. But I think it's a good thing to shelf for later.

Somehow. As the user A fall asleep for 20minutes new hosts we're added by another user. User A then press select all and starts to delete the hosts via the bulk action. The bulk action shows that x hosts were selected - which is the amount of hosts before sleeping for 20minutes on the all hosts page. As no host ids are send to the backend but search queries, it can always be, that the user deletes wrong hosts because in the meantime something has happened to the hosts. Maybe a user searches for all hosts with OS Alma and even excluded 4 hosts - in the meantime more hosts were added (with FAM) and they are deleted, too.

I have worked on a guard which uses digests of user ID, location ID, organization ID, and all host IDs a user has access. This digest is passed to the UI and forwarded to 'some' bulk actions like delete, power off which can really 'destroy' a datacenter managed by foreman. In the bulk action the hash is recalculated and only if the hash stays the same, the bulk action is executed. Not perfect, but I think it solves both issues.

In my point of view, the root cause is, that search queries (dynamic queries) are somehow send to the backend instead of host IDs. Only if the user would see which hosts are affected by a bulk action and sending these host IDs to the backend would really solve the issue .

nadjaheitmann · 2026-06-24T08:44:00Z

I think we can agree that this particular PR targets to fix the location(/organization?) scope mismatch issue between UI and the backend.

In parallel, I will create an RFC that outlines the broader problem with the dynamically scoped search queries.

sbernhard · 2026-06-24T10:34:14Z

PR with the guard I mentioned: #11038

nadjaheitmann · 2026-06-24T17:35:52Z

CI is green, this is ready for review :)

jeremylenz

Hi @nadjaheitmann

This seems to be working well! Sorry for the delay in review

I have 4 hosts in Default Org/Default Location (location_id 2)
I have 0 hosts in Loc2
on All Hosts page, I select all
in another tab, I change current location to Loc2
In the original tab, I perform a bulk action (I tried power state and build)

Even though current location is set to Loc2 (thru the other tab), the request still sends location_id 2 / Default Location, and it properly finds my hosts.

Basically it now behaves the way you would think it should - it respects the location you have in front of your eyes, rather than the more-recently-changed one in the other tab.

Before this patch, the Foreman UI did not send any information about taxonomies on the 'All Hosts' page for bulk actions. This could cause bulk actions to be applied to different hosts than the ones that were actually selected in the UI because the session in the backend would calculate the hosts independent from UI input. Assisted-by: OpenAI Codex

jeremylenz

Thanks @nadjaheitmann!

ACK 👍

nadjaheitmann · 2026-06-29T11:40:52Z

Thanks @jeremylenz , that was fast! Should we have a cherry-pick for 3.18 and 3.19? I am not sure if this is the best solution to the problem but I think it is better having this than the bug 🙃

nadjaheitmann · 2026-06-30T07:27:11Z

@jeremylenz @ianballou I have created an RFC in the community forum for awareness and further discussion: https://community.theforeman.org/t/rfc-destructive-bug-with-bulk-actions-on-all-hosts-ui-page/46920

jeremylenz · 2026-06-30T12:33:40Z

+1 to cherry-picking.

nadjaheitmann · 2026-07-01T11:46:24Z

+1 to cherry-picking.

Who decides that and when? What is the procedure here?

jeremylenz · 2026-07-01T13:29:07Z

The last I've heard is that in Foreman (unlike Katello) you're supposed to open your own cherry-pick PRs as soon as possible. @ofedoren is that right?

nadjaheitmann · 2026-07-06T06:48:07Z

The last I've heard is that in Foreman (unlike Katello) you're supposed to open your own cherry-pick PRs as soon as possible. @ofedoren is that right?

I was not aware of that, otherwise I would have opened a cherry-pick immediately. 3.18.2 was just released... Anyways, I am surprised about the low attention that this bug receives.

jeremylenz · 2026-07-07T14:34:24Z

perhaps I got the wrong "O" name and should have tagged @ogajduse ..

ogajduse · 2026-07-08T15:06:15Z

The last I've heard is that in Foreman (unlike Katello) you're supposed to open your own cherry-pick PRs as soon as possible. @ofedoren is that right?

I was not aware of that, otherwise I would have opened a cherry-pick immediately. 3.18.2 was just released... Anyways, I am surprised about the low attention that this bug receives.

Let's discuss that in the to-be-created community RFC.
See: https://community.theforeman.org/t/release-team-meeting-2026-07-08/46993#p-150278-other-8

Until we have a clear backporting process, I propose the PR author open a PR against the stable branch(es) they want to have the fix in.

pr-processor Bot added the Not yet reviewed label Jun 22, 2026

github-actions Bot added the UI label Jun 22, 2026

nadjaheitmann force-pushed the 39437_apply_taxononies_to_ui_bulk_actions branch 5 times, most recently from 06bed8b to 9adc2fe Compare June 22, 2026 09:30

ofedoren changed the title ~~39437 apply taxononies to UI bulk actions~~ Fixes #39437 - Apply taxonomies to UI bulk actions Jun 23, 2026

nadjaheitmann marked this pull request as draft June 23, 2026 11:53

ianballou reviewed Jun 23, 2026

View reviewed changes

pr-processor Bot removed the Not yet reviewed label Jun 23, 2026

nadjaheitmann force-pushed the 39437_apply_taxononies_to_ui_bulk_actions branch from 9adc2fe to 83b75b5 Compare June 24, 2026 12:37

nadjaheitmann marked this pull request as ready for review June 24, 2026 17:35

jeremylenz previously approved these changes Jun 25, 2026

View reviewed changes

nadjaheitmann dismissed jeremylenz’s stale review via 32f9854 June 26, 2026 07:13

nadjaheitmann force-pushed the 39437_apply_taxononies_to_ui_bulk_actions branch from 83b75b5 to 32f9854 Compare June 26, 2026 07:13

nadjaheitmann commented Jun 26, 2026

View reviewed changes

Comment thread app/controllers/api/v2/hosts_bulk_actions_controller.rb

nadjaheitmann force-pushed the 39437_apply_taxononies_to_ui_bulk_actions branch from 32f9854 to bdb805f Compare June 26, 2026 08:12

nadjaheitmann force-pushed the 39437_apply_taxononies_to_ui_bulk_actions branch from bdb805f to 41bf182 Compare June 26, 2026 08:13

jeremylenz approved these changes Jun 26, 2026

View reviewed changes

jeremylenz merged commit 34ee2c5 into theforeman:develop Jun 26, 2026
64 of 67 checks passed

maximiliankolb deleted the 39437_apply_taxononies_to_ui_bulk_actions branch June 29, 2026 05:32

Uh oh!

Conversation

nadjaheitmann commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nadjaheitmann commented Jun 22, 2026

Uh oh!

MariaAga commented Jun 23, 2026

Uh oh!

nadjaheitmann commented Jun 23, 2026

Uh oh!

jeremylenz commented Jun 23, 2026

Uh oh!

nadjaheitmann commented Jun 23, 2026

Uh oh!

jeremylenz commented Jun 23, 2026

Uh oh!

nadjaheitmann commented Jun 23, 2026

Uh oh!

ianballou commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianballou Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

nadjaheitmann Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremylenz Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

sbernhard commented Jun 23, 2026

Uh oh!

nadjaheitmann commented Jun 24, 2026

Uh oh!

sbernhard commented Jun 24, 2026

Uh oh!

nadjaheitmann commented Jun 24, 2026

Uh oh!

jeremylenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeremylenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nadjaheitmann commented Jun 29, 2026

Uh oh!

nadjaheitmann commented Jun 30, 2026

Uh oh!

jeremylenz commented Jun 30, 2026

Uh oh!

nadjaheitmann commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeremylenz commented Jul 1, 2026

Uh oh!

nadjaheitmann commented Jul 6, 2026

Uh oh!

jeremylenz commented Jul 7, 2026

Uh oh!

ogajduse commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nadjaheitmann commented Jun 22, 2026 •

edited

Loading

ianballou commented Jun 23, 2026 •

edited

Loading

nadjaheitmann Jun 24, 2026 •

edited

Loading

nadjaheitmann commented Jul 1, 2026 •

edited

Loading